Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add plugin-pyenv as an available function for fish #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pantuza
Copy link
Contributor

@pantuza pantuza commented May 26, 2017

This pull request has the following contributions:

  • Refactor the init script to become a function;
  • Create documentation of how to install and use plugin;

Copy link
Member

@scorphus scorphus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How willing would you be to test a rewrite I made a few months ago? It's available here: https://github.com/oh-my-fish/plugin-pyenv/tree/rehab


## Usage

Just add the following line in your ```config.fish``` file:
Copy link
Member

@scorphus scorphus Jun 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The beauty of plugins is them being able to just work out of the box, with the least possible being left up to the user. That means the init function of a plugin does all the witchery.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree. I did not find how to create this init. Since it is possible, this function may do that easier for users.


function pyenv-init --description "Pyenv initialization script for fish shell"

# Checks if directories already exists. If not, creates then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't pyenv take care of this? And is it really necessary to check every time a new session is created?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could also be solved by the init function. It is not necessary to run that on every new session.

set --query PYENV_ROOT; or set --local PYENV_ROOT $HOME/.pyenv

# Appends PYENV_ROOT directories to PATH variable
set PATH $PYENV_ROOT/shims $PYENV_ROOT/bin $PATH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this might be something related to making pyenv command available, but my experience tells me shims suffices. Not sure how it works on OSs other than OS X thou.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really do not know this behavior in other OSs. I just keep the way this plugin did.

@pantuza
Copy link
Contributor Author

pantuza commented Jun 13, 2017

The branch rehab seems really complete for me. Is there any problem at why it is not in the master?

It would be a good version of an official plugin.

@scorphus
Copy link
Member

All it lacks is a proper README.md and some test drive on a GNU/Linux box, which I could do later tonight.

@scorphus
Copy link
Member

And thank you for testing it! Much appreciated 👍

@brunobbbs
Copy link

The branch rehab also works great for me. Thanks!

@scorphus
Copy link
Member

Thanks for confirming it works, fellas!

Last night I had a chance to test it on a Linux box. It works there too only after a few tweaks. So rehab seems to work regardlessly of the install method – package manager or pyenv-installer.

But it still lacks a proper README.md. And I think we can gain from the one of this very PR. What do you think, @pantuza?

@pantuza
Copy link
Contributor Author

pantuza commented Jun 14, 2017

@scorphus I think that would be awesome. If you need any help, let me know!

@scorphus
Copy link
Member

scorphus commented Feb 5, 2018

Hey, dear fellas, @brunobbbs and @pantuza!

I've updated the rehab branch and created a pull request (#5). Can you please review it?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants